Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Added example utility using OOPS and IODA #553

Merged
merged 16 commits into from
Aug 9, 2023
Merged

Added example utility using OOPS and IODA #553

merged 16 commits into from
Aug 9, 2023

Conversation

CoryMartin-NOAA
Copy link
Contributor

This PR adds a very simple utility that will read in a IODA file, and for a specified group/variable, compute the mean and print it out. Nothing too fancy but is a nice C++ OOPS example.

@CoryMartin-NOAA CoryMartin-NOAA self-assigned this Aug 8, 2023
@CoryMartin-NOAA
Copy link
Contributor Author

@guillaumevernieres make sure I didn't screw up the merge conflicts

RussTreadon-NOAA
RussTreadon-NOAA reviewed Aug 8, 2023
View reviewed changes
Copy link
Contributor

@RussTreadon-NOAA RussTreadon-NOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for adding this oops example. This is a handy reference. Changes look good to my inexperienced eyes. I have not installed feature/oops_example on Hera or Orion and run ctests to ensure everything works a planned. Adding a CI label to this PR will do this for us.

@CoryMartin-NOAA CoryMartin-NOAA added the hera-GW-RT Queue for automated testing with global-workflow on Hera label Aug 8, 2023
@emcbot emcbot added hera-GW-RT-Running Automated testing with global-workflow running on Hera and removed hera-GW-RT Queue for automated testing with global-workflow on Hera labels Aug 8, 2023
guillaumevernieres
guillaumevernieres previously approved these changes Aug 8, 2023
View reviewed changes
Copy link
Contributor

@guillaumevernieres guillaumevernieres left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this @CoryMartin-NOAA . I have a minor cosmetic comment that you can decide if you want to address or not. Ready to merge, pending CI results. Code norm to be added in a separate PR.

@emcbot
Copy link

emcbot commented Aug 8, 2023

Automated Global-Workflow GDASApp Testing Results:
Machine: hera

Start: Tue Aug  8 15:04:31 UTC 2023 on hfe07
---------------------------------------------------
Build:                                  *FAILED*
Build: Failed at Tue Aug  8 15:32:10 UTC 2023
Build: see output at /scratch1/NCEPDEV/da/Cory.R.Martin/CI/GDASApp/workflow/PR/553/global-workflow/sorc/log.build

@emcbot emcbot added hera-GW-RT-Failed Automated testing with global-workflow failed on Hera and removed hera-GW-RT-Running Automated testing with global-workflow running on Hera labels Aug 8, 2023
@CoryMartin-NOAA CoryMartin-NOAA added hera-GW-RT Queue for automated testing with global-workflow on Hera and removed hera-GW-RT-Failed Automated testing with global-workflow failed on Hera labels Aug 9, 2023
@emcbot emcbot added hera-GW-RT-Running Automated testing with global-workflow running on Hera and removed hera-GW-RT Queue for automated testing with global-workflow on Hera labels Aug 9, 2023
@CoryMartin-NOAA
Copy link
Contributor Author

C++17 on Hera issue fixed by using a function that is 'older'. Not sure why that didn't build, I thought the compiler was new enough on Hera (this is concerning but we will cross that bridge later). Will try to loo into the failing tests some more now.

@CoryMartin-NOAA
Copy link
Contributor Author

@RussTreadon-NOAA I bet this is the culprit: https://github.com/JCSDA-internal/ufo/commit/92f78421da2713bb256f8f758381381060bad6a2

sensorScanPosition is float in our files, it needs to be int.

@emcbot
Copy link

emcbot commented Aug 9, 2023

Automated Global-Workflow GDASApp Testing Results:
Machine: hera

Start: Wed Aug  9 14:04:29 UTC 2023 on hfe07
---------------------------------------------------
Build:                                 *SUCCESS*
Build: Completed at Wed Aug  9 15:12:50 UTC 2023
---------------------------------------------------
Tests:                                  *Failed*
Tests: Failed at Wed Aug  9 16:03:00 UTC 2023
Tests: 89% tests passed, 6 tests failed out of 54
	1232 - test_gdasapp_util_ioda_example (Failed)
	1595 - test_gdasapp_atm_jjob_var_run (Failed)
	1596 - test_gdasapp_atm_jjob_var_final (Failed)
	1598 - test_gdasapp_atm_jjob_ens_run (Failed)
	1599 - test_gdasapp_atm_jjob_ens_final (Failed)
Tests: see output at /scratch1/NCEPDEV/da/Cory.R.Martin/CI/GDASApp/workflow/PR/553/global-workflow/sorc/gdas.cd/build/log.ctest

@emcbot emcbot added hera-GW-RT-Failed Automated testing with global-workflow failed on Hera and removed hera-GW-RT-Running Automated testing with global-workflow running on Hera labels Aug 9, 2023
@CoryMartin-NOAA
Copy link
Contributor Author

Apparently the CI testing does not download the JCSDA test data. @guillaumevernieres @RussTreadon-NOAA should it do that? or should I just manually include/download a single IODA file for this test? I could include one in our testdata tar file.

@RussTreadon-NOAA
Copy link
Contributor

Apparently the CI testing does not download the JCSDA test data. @guillaumevernieres @RussTreadon-NOAA should it do that? or should I just manually include/download a single IODA file for this test? I could include one in our testdata tar file.

Which approach is easier to implement now and maintain moving forward?

@CoryMartin-NOAA
Copy link
Contributor Author

Probably cloning the jcsda test data, but it might be overkill. I'm happy to do that if we think that's the easiest path forward.

@guillaumevernieres
Copy link
Contributor

Probably cloning the jcsda test data, but it might be overkill. I'm happy to do that if we think that's the easiest path forward.

@CoryMartin-NOAA , can you run your test with one of the soca ioda files? soca still uses git-lfs to store binaries.

@CoryMartin-NOAA
Copy link
Contributor Author

I went with @guillaumevernieres 's suggestion, it now uses one of the SOCA test files that have brightnessTemperature as a variable.

@CoryMartin-NOAA CoryMartin-NOAA added hera-GW-RT Queue for automated testing with global-workflow on Hera and removed hera-GW-RT-Failed Automated testing with global-workflow failed on Hera labels Aug 9, 2023
@emcbot emcbot added hera-GW-RT-Running Automated testing with global-workflow running on Hera and removed hera-GW-RT Queue for automated testing with global-workflow on Hera labels Aug 9, 2023
@RussTreadon-NOAA
Copy link
Contributor

test_gdasapp_util_ioda_example Passed on Orion following update at 55f1b6c.

@emcbot
Copy link

emcbot commented Aug 9, 2023

Automated Global-Workflow GDASApp Testing Results:
Machine: hera

Start: Wed Aug  9 17:46:08 UTC 2023 on hfe07
---------------------------------------------------
Build:                                 *SUCCESS*
Build: Completed at Wed Aug  9 18:34:49 UTC 2023
---------------------------------------------------
Tests:                                  *Failed*
Tests: Failed at Wed Aug  9 19:07:21 UTC 2023
Tests: 91% tests passed, 5 tests failed out of 54
	1607 - test_gdasapp_atm_jjob_var_run (Failed)
	1608 - test_gdasapp_atm_jjob_var_final (Failed)
	1610 - test_gdasapp_atm_jjob_ens_run (Failed)
	1611 - test_gdasapp_atm_jjob_ens_final (Failed)
Tests: see output at /scratch1/NCEPDEV/da/Cory.R.Martin/CI/GDASApp/workflow/PR/553/global-workflow/sorc/gdas.cd/build/log.ctest

@emcbot emcbot added hera-GW-RT-Failed Automated testing with global-workflow failed on Hera and removed hera-GW-RT-Running Automated testing with global-workflow running on Hera labels Aug 9, 2023
@CoryMartin-NOAA
Copy link
Contributor Author

The failures are unrelated to this PR it seems, but I defer to @RussTreadon-NOAA and @guillaumevernieres if we want to try to fix them before approving/merging this.

@RussTreadon-NOAA
Copy link
Contributor

issue #554 has been opened to report the var_run and ens_run failures. A separate PR can be opened to resolve whatever isn't right. No need to hold up this PR for what seems to be an ioda or ufo problem.

guillaumevernieres
guillaumevernieres approved these changes Aug 9, 2023
View reviewed changes
Copy link
Contributor

@guillaumevernieres guillaumevernieres left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been voting for ignoring the atmos ctest for the last few pr's! Looks ready to merge. Thanks again @CoryMartin-NOAA .

@CoryMartin-NOAA CoryMartin-NOAA merged commit f601d0c into develop Aug 9, 2023
11 checks passed
@CoryMartin-NOAA CoryMartin-NOAA deleted the feature/oops_example branch August 9, 2023 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RussTreadon-NOAA RussTreadon-NOAA RussTreadon-NOAA left review comments

@guillaumevernieres guillaumevernieres guillaumevernieres approved these changes

@emilyhcliu emilyhcliu Awaiting requested review from emilyhcliu

@ADCollard ADCollard Awaiting requested review from ADCollard

@kevindougherty-noaa kevindougherty-noaa Awaiting requested review from kevindougherty-noaa

@DavidNew-NOAA DavidNew-NOAA Awaiting requested review from DavidNew-NOAA

Assignees

@CoryMartin-NOAA CoryMartin-NOAA

Labels
hera-GW-RT-Failed Automated testing with global-workflow failed on Hera
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@CoryMartin-NOAA @emcbot @RussTreadon-NOAA @guillaumevernieres